Skip to content

Conversation

@mkannwischer
Copy link
Contributor

@mkannwischer mkannwischer commented Jan 15, 2026

Restructure MLD_INLINE and MLD_ALWAYS_INLINE into separate blocks,
each independently guarded:

MLD_INLINE:

  • MSVC: __inline
  • C99 or inline macro defined: inline
  • GCC/Clang C90: __attribute__((unused)) to silence warnings
  • Other C90: empty

MLD_ALWAYS_INLINE:

  • MSVC: __forceinline
  • GCC/Clang C99+: MLD_INLINE __attribute__((always_inline))
  • Other: MLD_INLINE (no forced inlining)

This fixes two issues:

  1. If inline was defined as a macro before including sys.h, the MSVC
    check was bypassed and __attribute__ was used, causing MSVC failures.
  2. For non-MSVC/GCC/Clang compilers like IAR, __attribute__ was used
    unconditionally, causing build failures.

Also fix typo: defined(clang) -> defined(__clang__)

@mkannwischer
Copy link
Contributor Author

mkannwischer commented Jan 15, 2026

@gilles-peskine-arm, I hope that this solves your issues with MSVC and IAR. Could you try it out?

Btw, feel free to tag @pq-code-package/pqcp-mldsa-native-admin for issues like this in the future. We'll try to help if we have some spare cycles. This time we only saw it coincidentally.

@mkannwischer mkannwischer force-pushed the fix-mld-inline branch 3 times, most recently from 61db8f5 to fb8b926 Compare January 15, 2026 02:16
@mkannwischer mkannwischer marked this pull request as ready for review January 15, 2026 03:30
@mkannwischer mkannwischer requested a review from a team as a code owner January 15, 2026 03:30
Copy link
Contributor

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gilles-peskine-arm for noting this, and @mkannwischer for analyzing and fixing. LGTM.

Restructure `MLD_INLINE` and `MLD_ALWAYS_INLINE` into separate blocks,
each independently guarded:

`MLD_INLINE`:
- MSVC: `__inline`
- C99 or `inline` macro defined: `inline`
- GCC/Clang C90: `__attribute__((unused))` to silence warnings
- Other C90: empty

`MLD_ALWAYS_INLINE`:
- MSVC: `__forceinline`
- GCC/Clang C99+: `MLD_INLINE __attribute__((always_inline))`
- Other: `MLD_INLINE` (no forced inlining)

This fixes two issues:
1. If `inline` was defined as a macro before including sys.h, the MSVC
   check was bypassed and `__attribute__` was used, causing MSVC failures.
2. For non-MSVC/GCC/Clang compilers like IAR, `__attribute__` was used
   unconditionally, causing build failures.

Also fix typo: `defined(clang)` -> `defined(__clang__)`

Signed-off-by: Matthias J. Kannwischer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working needs-mlkem-native-port

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants